Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move FD_SETSIZE to configure.ac #1135

Open
wants to merge 1 commit into
base: riscv
Choose a base branch
from
Open

Conversation

borneoa
Copy link
Contributor

@borneoa borneoa commented Sep 23, 2024

The commit from the pull request #644 sets the macro FD_SETSIZE to 128 for every host OS, while it is needed on Windows only. Note that this macro has a much higher value on other OS and after this commit it get reduced to 128 only.

Since in configure.ac is already present a hook to change the compile defines for Windows, move there also FD_SETSIZE.

I have pushed in upstream OpenOCD the patch https://review.openocd.org/c/openocd/+/8503 that, together with this pull request, aligns the two repositories on this topic.
I will wait for review and eventual merge of this pull request before merging the corresponding patch upstream. Of course, the review process is open upstream too.
Hope this helps the sync between the two repositories.

The commit from the pull request riscv-collab#644 sets the macro FD_SETSIZE to
128 for every host OS, while it is needed on Windows only.
Note that this macro has a much higher value on other OS and after
this commit it get reduced to 128 only.

Since in configure.ac is already present a hook to change the
compile defines for Windows, move there also FD_SETSIZE.

Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Copy link
Collaborator

@en-sc en-sc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch! LGTM.
@JanMatCodasip, @MarekVCodasip, please take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants